Resolver: Parallelize the import resolution loop by LorrensP-2158466 · Pull Request #158845 · rust-lang/rust

LorrensP-2158466 · 2026-07-06T08:22:15Z

Follow up of #159440. This pr implements the parallel part of par_for_each_slice. And implements DynSend and DynSync for RefOrMut and CmCell to make the call to par_for_each_slice compile.

This is the bare minimum to make the parallel loop work and is not at all optimized, this will follow :).

r? @petrochenkov

LorrensP-2158466 · 2026-07-06T08:22:55Z

Lets see what CI says.

I have tried to add comments to changes to show my thought process behind them, but i'll make another pass here as well to be sure.

petrochenkov · 2026-07-06T16:24:33Z

@bors try @rust-timer queue

…r=<try> Resolver: Parallelize the import resolution loop

rust-bors · 2026-07-06T18:42:55Z

☀️ Try build successful (CI)
Build commit: 3df1fed (3df1fed3aaf1081aa59cfe2228eed289a2ef744e)
Base parent: 3c00c96 (3c00c96d3af4d5b5e101e56cc161a608b21366ee)

LorrensP-2158466 · 2026-07-27T17:39:36Z

I've finally read the safety comments in detail, they look more convincing now.

I'll think that I will extend them a bit and add a comment to the ref_mut module as to why we want these (weird) data structures.

LorrensP-2158466 · 2026-07-27T19:30:27Z

Updated the comments and used RefCell instead of the manual impl, @rustbot ready.

petrochenkov · 2026-07-28T11:09:40Z

Benchmarking the changes in #160064.

LorrensP-2158466 · 2026-07-28T12:20:33Z

I'm also tempted to add extra methods for CmCell and CmRefCell that accept a &mut Resolver as a "token" that we can do mutations without checking assert_speculative.

LorrensP-2158466 · 2026-07-28T14:08:33Z

https://github.com/LorrensP-2158466/ref_mut/tree/main has the code for the miri tests, as you will see (and the comments in this pr explain) CmRefCell is not entirely safe.

LorrensP-2158466 · 2026-07-28T15:53:08Z

decided to create a CmToken instead of using &mut Resolver in these things, seems cleaner to me at least. @RutBot ready.

petrochenkov · 2026-07-28T16:10:16Z

#160064 already showed regressions, so let's benchmark and land this piece by piece, starting from ResolutionTable.

Also blocked on #160090.

petrochenkov · 2026-07-28T16:28:25Z

the assert_speculative flag (which needs to be atomic, most likely).

At least right now it probably doesn't need to be atomic though.
assert_speculative = true, par_for_each_slice start, par_for_each_slice end, and assert_speculative = false all happens on the same thread, so there's happens-before relationship between them.
And par_for_each_slice's start and end are supposed to synchronize with everything that happens between them on all the used threads, as I understand.

petrochenkov · 2026-07-28T16:30:29Z

decided to create a CmToken instead of using &mut Resolver in these things, seems cleaner to me at least

Again, we'll need to benchmark whether eliding this boolean assert even matters, considering that it's not even atomic.

LorrensP-2158466 · 2026-07-29T14:15:59Z

I'll split the CmRefCell changes as well (first without token). Okey with you?

petrochenkov · 2026-07-29T14:20:37Z

I'll split the CmRefCell changes as well (first without token). Okey with you?

Yes, after the ResolutionTable lands

LorrensP-2158466 · 2026-08-19T08:13:17Z

Rebased to fix conflicts and cleanup the PR, it now contains:

Parallel loop, together with impls of DynSync and usage of RwLock in some cases
The RwLock union impl from Implement rustc_datastructures::RwLock as union over RefCell and parking_lot::RwLock. #159452

@rustbot ready.

petrochenkov · 2026-08-19T08:41:44Z

The RwLock union impl from Implement rustc_datastructures::RwLock as union over RefCell and parking_lot::RwLock. #159452

I don't think we'll merge this part, given the unconvincing perf results from the linked PR and the amount of complexity/unsafety.
Could you remove it from this PR? We could benchmark it later separately (but probably still not merge).

petrochenkov · 2026-08-19T10:14:38Z

-                            scope = next.get();
-                            macro_rules_scope.set(scope);
+                            scope = *next.borrow();
+                            *macro_rules_scope.borrow_mut() = scope;


I'm not sure this is algorithmically correct.
We may need to protect a larger piece of logic by a lock, maybe the whole loop.
I need to think (but I'm busy this and next week).

View changes since the review

I am not entirely familiar with the chained scopes of macro_rules!, but I can assume there are no cycles, so this should be correct.

Example chain:

A -> B -> C -> D -> E

If multiple threads try to compress the path A -> E starting at A, then no matter the ordering, they will always end up at E.

t1 reads node A and writes on that node B, then t2 will read B.

t1 and t2 read node A, both will then write B, which is the result we expect.

I see it as a recursive property as well, so it will hold until both threads read E in their last step.

If multiple threads are compressing on the same path, their results will be the same as well. For example: t1 compresses A -> E and t2 will compress C -> E.
Since both only overwrite their respective nodes, A for t1 and C for t2, they can never interfere with each other. It doesn't matter what t2 is doing on its path, because for t1 the full path still "exists", if t2 managed to overwrite C before t1 gets to it, it just does one step less.

I'm still not convinced.
Assume we have a slow thread t_slow and a fast thread t_fast.

The slow thread arrives first and reads a MacroRulesScope::Invocation in scope = *next.borrow();.

Then the fast thread arrives and compresses the whole chain eliminating the MacroRulesScope::Invocation from the step 1 in particular

Then the slow thread continues and does macro_rules_scope.set(scope); putting MacroRulesScope::Invocation back into the macro_rules_scope again.

In the meantime the fast thread continues assuming that the chain is fully compressed and there're no expanded MacroRulesScope::Invocations -> "never observe invocation scopes for macros that were already expanded" invariant from the comment above is broken.

… parallel loop for import resolution. + impl `DynSend`/`DynSync` for all relevant data structures + safety comments.

…` and `RwLock` instead`

rustbot · 2026-08-26T07:58:50Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

LorrensP-2158466 · 2026-08-26T07:58:57Z

Force-pushed to keep up to date (and remove RwLock change) and address both comments + the removal of Cache(Ref)Cell.

i'll wait on #158845 (comment), so i'll mark this @rustbot ready.

petrochenkov · 2026-09-03T13:07:03Z

        }
    }

-    /// A wrapper around a [`RefCell`] that only allows writes (mutable borrows) based on a condition in the resolver.


Lost a comment.

View changes since the review

petrochenkov · 2026-09-03T13:41:37Z

-                            scope = next.get();
-                            macro_rules_scope.set(scope);
+                            scope = *next.borrow();
+                            *macro_rules_scope.borrow_mut() = scope;


I'm still not convinced.
Assume we have a slow thread t_slow and a fast thread t_fast.

The slow thread arrives first and reads a MacroRulesScope::Invocation in scope = *next.borrow();.

Then the fast thread arrives and compresses the whole chain eliminating the MacroRulesScope::Invocation from the step 1 in particular

Then the slow thread continues and does macro_rules_scope.set(scope); putting MacroRulesScope::Invocation back into the macro_rules_scope again.

In the meantime the fast thread continues assuming that the chain is fully compressed and there're no expanded MacroRulesScope::Invocations -> "never observe invocation scopes for macros that were already expanded" invariant from the comment above is broken.

petrochenkov · 2026-09-03T13:42:42Z

Could you move the Cache(Ref)Cell parts to a separate PR so they could be benchmarked separately?
@rustbot author

rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 6, 2026

rustbot assigned petrochenkov Jul 6, 2026

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 6, 2026

LorrensP-2158466 commented Jul 6, 2026

View reviewed changes

Comment thread compiler/rustc_data_structures/src/sync/parallel.rs Outdated

LorrensP-2158466 commented Jul 6, 2026

View reviewed changes

Comment thread compiler/rustc_resolve/src/build_reduced_graph.rs Outdated

LorrensP-2158466 commented Jul 6, 2026

View reviewed changes

Comment thread compiler/rustc_resolve/src/imports.rs Outdated

LorrensP-2158466 commented Jul 6, 2026

View reviewed changes

Comment thread compiler/rustc_resolve/src/imports.rs

LorrensP-2158466 commented Jul 6, 2026

View reviewed changes

Comment thread compiler/rustc_resolve/src/imports.rs Outdated

LorrensP-2158466 commented Jul 6, 2026

View reviewed changes

Comment thread compiler/rustc_resolve/src/imports.rs Outdated

This comment has been minimized.

Sign in to view

LorrensP-2158466 commented Jul 6, 2026

View reviewed changes

Comment thread compiler/rustc_resolve/src/lib.rs Outdated

LorrensP-2158466 commented Jul 6, 2026

View reviewed changes

Comment thread compiler/rustc_resolve/src/lib.rs Outdated

LorrensP-2158466 commented Jul 6, 2026

View reviewed changes

Comment thread compiler/rustc_resolve/src/lib.rs Outdated

This comment has been minimized.

Sign in to view

LorrensP-2158466 force-pushed the parallel-import-resolution branch from 17bd1cb to 9aa1069 Compare July 6, 2026 10:48

This comment has been minimized.

Sign in to view

LorrensP-2158466 force-pushed the parallel-import-resolution branch from 9aa1069 to a45ba86 Compare July 6, 2026 11:31

petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 6, 2026

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 6, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Jul 6, 2026

Auto merge of #158845 - LorrensP-2158466:parallel-import-resolution, …

3df1fed

…r=<try> Resolver: Parallelize the import resolution loop

petrochenkov reviewed Jul 6, 2026

View reviewed changes

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 6, 2026

This comment has been minimized.

Sign in to view

LorrensP-2158466 commented Jul 27, 2026

View reviewed changes

Comment thread compiler/rustc_resolve/src/lib.rs Outdated

petrochenkov reviewed Jul 28, 2026

View reviewed changes

Comment thread compiler/rustc_resolve/src/lib.rs Outdated

This comment has been minimized.

Sign in to view

LorrensP-2158466 mentioned this pull request Jul 28, 2026

Resolver: split module resolutions into local and external resolutions #160099

Merged

This comment has been minimized.

Sign in to view

This was referenced Jul 31, 2026

Resolver: Introduce CmRef which has a speclative borrow variant for CmRefCell #160271

Merged

Resolver: add checked methods for Cm(Ref)Cell #160345

Merged

This comment has been minimized.

Sign in to view

petrochenkov reviewed Aug 19, 2026

View reviewed changes

Comment thread compiler/rustc_resolve/src/lib.rs Outdated

Comment thread compiler/rustc_resolve/src/lib.rs Outdated

petrochenkov reviewed Aug 19, 2026

View reviewed changes

LorrensP-2158466 added 3 commits August 26, 2026 09:56

Implement the parallel part of par_for_each_slice, thus achieving a…

17bc461

… parallel loop for import resolution. + impl `DynSend`/`DynSync` for all relevant data structures + safety comments.

remove CacheCell and CacheRefCell aliases and directly use `Mutex…

51a52fd

…` and `RwLock` instead`

address review

8b5580f

petrochenkov reviewed Sep 3, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

LorrensP-2158466 commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LorrensP-2158466 commented Jul 6, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

petrochenkov commented Jul 6, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rust-bors Bot commented Jul 6, 2026

Uh oh!

This comment has been minimized.

LorrensP-2158466 commented Jul 27, 2026

Uh oh!

LorrensP-2158466 commented Jul 27, 2026

Uh oh!

Uh oh!

petrochenkov commented Jul 28, 2026

Uh oh!

Uh oh!

LorrensP-2158466 commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LorrensP-2158466 commented Jul 28, 2026

Uh oh!

LorrensP-2158466 commented Jul 28, 2026

Uh oh!

This comment has been minimized.

petrochenkov commented Jul 28, 2026

Uh oh!

petrochenkov commented Jul 28, 2026

Uh oh!

petrochenkov commented Jul 28, 2026

Uh oh!

LorrensP-2158466 commented Jul 29, 2026

Uh oh!

petrochenkov commented Jul 29, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

LorrensP-2158466 commented Aug 19, 2026

Uh oh!

petrochenkov commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

petrochenkov Aug 19, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LorrensP-2158466 Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

petrochenkov Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

LorrensP-2158466 commented Jul 6, 2026 •

edited

Loading

LorrensP-2158466 commented Jul 28, 2026 •

edited

Loading

petrochenkov commented Aug 19, 2026 •

edited

Loading

petrochenkov Aug 19, 2026 •

edited by rustbot

Loading

petrochenkov Sep 3, 2026 •

edited

Loading

petrochenkov Sep 3, 2026 •

edited by rustbot

Loading

petrochenkov Sep 3, 2026 •

edited

Loading